fix(tm2/amino): preserve nil_elements in genproto2 by D4ryl00 · Pull Request #5569 · gnolang/gno

D4ryl00 · 2026-04-22T08:06:40Z

Summary

Fix a genproto2 regression introduced by #5282 e8aac7575f (feat(amino): add genproto2).

After regenerating pb3_gen.go from master, gnoland consensus started failing deterministically because types.Commit.Precommits uses amino:"nil_elements", and the generated genproto2 decoder did not preserve positional nil entries in unpacked pointer lists.

In the failing path:

a commit with one missing validator precommit was marshaled with a zero-length ByteLength entry for that validator
genproto2 unmarshal reconstructed that entry incorrectly
the next proposal’s LastCommit then failed block validation with wrong LastCommit
the proposer rejected its own block and the chain wedged

Root Cause

Marshal was already correct: for unpacked pointer lists with amino:"nil_elements", genproto2 encodes a nil element as an empty ByteLength value (0x00), matching the reflect codec.

The bug was in generated unmarshal: it did not turn that empty entry back into nil. Instead, it decoded it as a non-nil zero-value element.

For types.Commit.Precommits, that is consensus-critical because the list is positional. A missing precommit must remain nil; turning it into a zero-value CommitSig makes the reconstructed commit invalid, which is why the next proposal failed with wrong LastCommit.

The fix makes generated unmarshal match the reflect codec again: empty entries decode to nil, non-empty entries stay on the generated fast path, and anyDepth is preserved.

Why This Fix

Commit.Precommits is positional and uses amino:"nil_elements". Losing nil entries or changing their decode semantics is not just cosmetic; it changes the meaning of the commit.

The final implementation keeps genproto2 behavior aligned with the reflect codec while preserving the generated fast path and existing depth-guard semantics.

Signed-off-by: D4ryl00 <d4ryl00@gmail.com>

Gno2D2 · 2026-04-22T08:07:12Z

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

IGNORE the bot requirements for this PR (force green CI check)
The pull request description provides enough details

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:

Fix any issues flagged by automated checks.
Follow the Contributor Checklist to ensure your PR is ready for review.
- Add new tests, or document why they are unnecessary.
- Provide clear examples/screenshots, if necessary.
- Update documentation, if required.
- Ensure no breaking changes, or include BREAKING CHANGE notes.
- Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: D4ryl00/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 User jefft0 already reviewed PR 5569 with state APPROVED
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)
If
🟢 Condition met
└── 🟢 On every pull request
Can be checked by

Any user with comment edit permission
The pull request description provides enough details
If
🟢 Condition met
└── 🟢 And
    ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors)
    └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])
Can be checked by

team core-contributors

codecov · 2026-04-22T08:08:55Z

Codecov Report

❌ Patch coverage is 88.39779% with 42 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
tm2/pkg/amino/genproto2/gen_unmarshal.go	70.00%	20 Missing and 4 partials ⚠️
tm2/pkg/amino/tests/pb3_gen.go	95.48%	8 Missing and 4 partials ⚠️
tm2/pkg/bft/types/pb3_gen.go	62.50%	4 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

jefft0

This PR has tests for the fix described in the description. CI checks pass. Ready for core devs to review. Urgent fix for the stability of the chain.

Signed-off-by: D4ryl00 <d4ryl00@gmail.com>

jaekwon · 2026-04-24T04:47:28Z

taking a look now

jaekwon · 2026-04-24T09:06:17Z

Added alt with more fixes: #5590

fix: unmarshal amino nil pointers

3d3524b

Signed-off-by: D4ryl00 <d4ryl00@gmail.com>

github-project-automation Bot added this to 💪 Bounties & Worx and 🧙‍♂️Gno.land development Apr 22, 2026

github-project-automation Bot moved this to Triage in 🧙‍♂️Gno.land development Apr 22, 2026

D4ryl00 marked this pull request as draft April 22, 2026 08:06

github-actions Bot assigned D4ryl00 Apr 22, 2026

github-actions Bot added 📦 🌐 tendermint v2 Issues or PRs tm2 related amino Issues and PRs related to amino labels Apr 22, 2026

D4ryl00 marked this pull request as ready for review April 22, 2026 08:53

Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Apr 22, 2026

D4ryl00 changed the title ~~fix: unmarshal amino nil pointers~~ fix(tm2/amino): preserve nil_elements in genproto2 Apr 22, 2026

jefft0 approved these changes Apr 22, 2026

View reviewed changes

Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Apr 22, 2026

jefft0 added this to 🤝🏻 Partner: Berty Apr 22, 2026

jefft0 moved this to Other in review in 🤝🏻 Partner: Berty Apr 22, 2026

aeddi added the a/gnops DevOps, Valopers, NetOps, Infra, Monitoring, Coordination team label Apr 22, 2026

ajnavarro approved these changes Apr 22, 2026

View reviewed changes

Comment thread tm2/pkg/amino/genproto2/gen_unmarshal.go Outdated

github-project-automation Bot moved this to In Progress in 💪 Bounties & Worx Apr 22, 2026

chore: improve comment

6cab286

Signed-off-by: D4ryl00 <d4ryl00@gmail.com>

ltzmaxwell mentioned this pull request Apr 23, 2026

WIP Feat/maxwell/genproto2 testability #5575

Open

ajnavarro requested a review from jaekwon April 23, 2026 10:04

nemanjantic moved this from Triage to In Review in 🧙‍♂️Gno.land development Apr 23, 2026

jaekwon mentioned this pull request Apr 24, 2026

fix(amino): nil_elements + AminoMarshaler repr-zeroness + BinFixed64 + cross-codec parity harness (alt of #5569) #5590

Open

jaekwon closed this Apr 24, 2026

github-project-automation Bot moved this from In Progress to Done in 💪 Bounties & Worx Apr 24, 2026

github-project-automation Bot moved this from Other in review to Done in 🤝🏻 Partner: Berty Apr 24, 2026

github-project-automation Bot moved this from In Review to Done in 🧙‍♂️Gno.land development Apr 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tm2/amino): preserve nil_elements in genproto2#5569

fix(tm2/amino): preserve nil_elements in genproto2#5569
D4ryl00 wants to merge 2 commits intognolang:masterfrom
D4ryl00:fix/amino-nil-elements-commit

D4ryl00 commented Apr 22, 2026 •

edited

Loading

Uh oh!

Gno2D2 commented Apr 22, 2026 •

edited

Loading

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Then

If

Then

If

Can be checked by

If

Can be checked by

Uh oh!

codecov Bot commented Apr 22, 2026 •

edited

Loading

Uh oh!

jefft0 left a comment

Uh oh!

Uh oh!

jaekwon commented Apr 24, 2026

Uh oh!

jaekwon commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

D4ryl00 commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Why This Fix

Uh oh!

Gno2D2 commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠 PR Checks Summary

Manual Checks (for Reviewers):

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Then

If

Then

If

Can be checked by

If

Can be checked by

Uh oh!

codecov Bot commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jefft0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jaekwon commented Apr 24, 2026

Uh oh!

jaekwon commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

D4ryl00 commented Apr 22, 2026 •

edited

Loading

Gno2D2 commented Apr 22, 2026 •

edited

Loading

codecov Bot commented Apr 22, 2026 •

edited

Loading